-
Couldn't load subscription status.
- Fork 5
feat: registration, delegation, and MIR history account endpoints #292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: William Hankins <[email protected]>
…ert-processing' into whankinsiv/reg-deleg-and-mir-rest-handlers
|
Awaiting the review/merge of #272 before moving to ready for review. |
…and-mir-rest-handlers
Signed-off-by: William Hankins <[email protected]>
Signed-off-by: William Hankins <[email protected]>
Signed-off-by: William Hankins <[email protected]>
| Err(e) => { | ||
| return Ok(RESTResponse::with_text( | ||
| 500, | ||
| &format!("Failed to encode pool ID: {e}"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bigger change than I think makes sense in this PR, but in other projects I usually implement an API error to deal with the boilerplate of friendly REST error messages:
pub struct AppError(u16, anyhow::Error);
impl From<AppError> for RESTResponse {
fn from(value: AppError) -> Self {
Self::with_text(value.0, value.1.to_string())
}
}
impl<E> From<E> for AppError
where
E: Into<anyhow::Error>,
{
fn from(value: E) -> Self {
Self(500, value.into())
}
}It lets you lean on anyhow, and write calls to fallible methods in a much briefer way:
let pool_id = r.pool.to_bech32_with_hrp("pool").context("Failed to encode pool id")?;There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created #313 so that this refactor can be implemented in a follow up PR. This will clean up the module significantly.
| Message::StateQueryResponse(StateQueryResponse::Accounts( | ||
| AccountsStateQueryResponse::NotFound, | ||
| )) => { | ||
| return Err(anyhow::anyhow!("Account not found")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this result in a 404 error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5eb101a to properly return 404 errors on NotFound. Thanks for catching this.
Signed-off-by: William Hankins <[email protected]>
|
Merging since all feedback has been addressed. I've created a follow up issue for Simon's suggestion about introducing an |
This PR implements 3 of the 10 remaining account related endpoints described in #256:
/accounts/{stake_address}/registrations- Querieshistorical_accounts_statefor registration history, then fetchesTxHashvalues fromchain_storeusing eachTxIdentifierto construct the Blockfrost aligned response./accounts/{stake_address}/delegations- Querieshistorical_accounts_statefor delegation history, resolvingTxHashviachain_store./accounts/{stake_address}/mirs- Querieshistorical_accounts_statefor MIR history, resolvingTxHashviachain_store.Main changes:
rest_blockfrostinitialization.handle_account_registrations_blockfrost,handle_account_delegations_blockfrost, andhandle_account_mirs_blockfrosttohandlers/accounts.rs.Notes:
accounts_statetohistorical_accounts_statealthough this would require passing the per stake address distribution on the message bus. Alternatively, we could rely on a second query in the rest handler to retrieve the account epoch stake fromaccounts_stateunder the assumption thatspdd-retention-epochsis configured to store all epochs.Next steps:
historical_accounts_stateand implement the corresponding REST handler.